Skip to content

More chain code refactoring - #7118

Merged
jieyouxu merged 3 commits into
rust-lang:mainfrom
matthewhughes934:chain-refactor-3
Sep 16, 2026
Merged

jieyouxu merged 3 commits into
rust-lang:mainfrom
matthewhughes934:chain-refactor-3

Conversation

@matthewhughes934

@matthewhughes934 matthewhughes934 commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

I'm continuing to try and make this code more understandable as I walk
through it. All the changes here should be pure refactoring, so not
behavioural changes.

  • refactor(chain): refactor chain parsing

    Some general code cleanup, things like:

    • Replace some ref patterns with & ones
    • Only pass down only required field to helper function
    • Add comments for span attribute
  • refactor(chain): drop CommentPosition

    I found the naming in this type quite confusing: all it was tracking was
    "does this comment belong on the same line", so use a bool for this
    purpose.

  • Add some more chain comment tests

    So that I can be confidence that my future refactors aren't changing
    some untested behaviour.

  • I did not use an LLM to create a change in this PR.

  • I used an LLM to create a change in this PR, and I have explained below how it was used.

@rustbot rustbot added the S-waiting-on-review Status: awaiting review from the assignee but also interested parties. label Sep 12, 2026
@jieyouxu jieyouxu self-assigned this Sep 14, 2026

@jieyouxu jieyouxu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, some nits
@rustbot author

View changes since this review

Comment thread src/chains.rs Outdated
Comment thread src/chains.rs
Comment on lines +165 to +168
parent //comment
.1
.2
.3;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks funny but yeah pre-existing.

@rustbot rustbot added S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: awaiting review from the assignee but also interested parties. labels Sep 15, 2026
@rustbot

rustbot commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Some general code cleanup, things like:

* Replace some `ref` patterns with `&` ones
* Only pass down only required field to helper function
* Add comments for span attribute
I found the naming in this type quite confusing: all it was tracking was
"does this comment belong on the same line", so update the name to be
clearer (I actually stole the naming from `lists::ListItemCommentStyle`
in this same crate)
So that I can be confidence that my future refactors aren't changing
some untested behaviour.

@jieyouxu jieyouxu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jieyouxu
jieyouxu added this pull request to the merge queue Sep 16, 2026
Merged via the queue into rust-lang:main with commit ffd3011 Sep 16, 2026
28 checks passed
@rustbot rustbot added release-notes Needs an associated changelog entry and removed S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. labels Sep 16, 2026
@matthewhughes934
matthewhughes934 deleted the chain-refactor-3 branch September 16, 2026 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes Needs an associated changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants